-
Notifications
You must be signed in to change notification settings - Fork 8
fix: Update URL construction for Android instant app redirection #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: kryptocodes <[email protected]>
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ReclaimProofRequest
participant AndroidLauncher
User->>ReclaimProofRequest: call redirectToInstantApp()
ReclaimProofRequest->>ReclaimProofRequest: encode template
ReclaimProofRequest->>ReclaimProofRequest: build intent URI with template
ReclaimProofRequest->>User: return/trigger intent URI
User->>AndroidLauncher: Android receives intent URI and launches app
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.38.6)src/Reclaim.ts[ ... [truncated 12152 characters] ... }, Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/Reclaim.ts (1)
735-739
: Avoid logging full redirect URL (PII leakage risk)
instantAppUrl
embeds JSON template (context, parameters, etc.). Logging it can leak PII. Log minimal context instead.- logger.info('Redirecting to Android instant app: ' + instantAppUrl); + logger.info(`Redirecting to Android instant app (sessionId=${this.sessionId})`);
🧹 Nitpick comments (1)
src/Reclaim.ts (1)
737-739
: Optional: use replace() to avoid back-navigation and consider canonical intent fallback
- Using
window.location.replace
prevents users from landing back on the intermediate page via back button.- For broader compatibility, consider an intent URI that uses
S.browser_fallback_url
(encoded) instead of query params to Play. This is a common, well-supported pattern.- window.location.href = instantAppUrl; + window.location.replace(instantAppUrl);Example alternative intent format (for future refactor):
intent://<host><path>?<q>#Intent;scheme=https;S.browser_fallback_url=<encoded_play_store_url>;end;
Signed-off-by: kryptocodes <[email protected]>
src/Reclaim.ts
Outdated
@@ -730,8 +730,7 @@ export class ReclaimProofRequest { | |||
let template = encodeURIComponent(JSON.stringify(this.templateData)); | |||
template = replaceAll(template, '(', '%28'); | |||
template = replaceAll(template, ')', '%29'); | |||
|
|||
const instantAppUrl = `https://share.reclaimprotocol.org/verify/?template=${template}`; | |||
const instantAppUrl = `intent://details?id=org.reclaimprotocol.app&launch=true&template=${template}&referrer=Z#Intent;scheme=market;action=android.intent.action.VIEW;package=com.android.vending;end;`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
referer as Z why?
Description
Fixed Android instant app redirect to use proper intent scheme for redirection. The previous implementation used a direct URL to the web verification page, but this change wraps it in an Android intent that will redirect users to instant app (also remove the open in browser app)
Testing (ignore for documentation update)
Type of change
Checklist:
Additional Notes:
If the device doesn't support instant app it will redirect to the playstore
Summary by CodeRabbit